Make get_norm_layer repr test tolerant of PyTorch bias= field#8880
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates a test file to handle PyTorch version differences in norm-layer string representations. It adds a Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/networks/layers/test_get_layers.py (1)
22-24: ⚡ Quick winAdd a Google-style docstring to the new helper.
_strip_bias_fieldis a new definition and should document args/returns in Google style per repo rules.Proposed patch
def _strip_bias_field(text: str) -> str: - # PyTorch >= 2.13 adds an optional ``bias=`` field to norm-layer repr; drop it for version stability. + """Remove optional PyTorch norm-layer ``bias=...`` repr fragment. + + Args: + text: Layer string representation to normalize. + + Returns: + The normalized representation with optional `, bias=True|False` removed. + + Raises: + None. + """ return re.sub(r",\s*bias=(?:True|False)", "", text)As per coding guidelines: “Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/networks/layers/test_get_layers.py` around lines 22 - 24, Add a Google-style docstring to the helper function _strip_bias_field describing its purpose, its arguments, and return value: explain that it accepts text (str) and returns a str with any optional ", bias=True" or ", bias=False" substrings removed for PyTorch repr stability; include an Args section listing text: str and a Returns section describing the cleaned string; no exceptions need to be documented.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/networks/layers/test_get_layers.py`:
- Around line 22-24: Add a Google-style docstring to the helper function
_strip_bias_field describing its purpose, its arguments, and return value:
explain that it accepts text (str) and returns a str with any optional ",
bias=True" or ", bias=False" substrings removed for PyTorch repr stability;
include an Args section listing text: str and a Returns section describing the
cleaned string; no exceptions need to be documented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa65659f-2616-4b76-b9ae-2a04645ce360
📒 Files selected for processing (1)
tests/networks/layers/test_get_layers.py
PyTorch >= 2.13 adds an optional 'bias=' token to GroupNorm/InstanceNorm __repr__, breaking the exact-string match in test_norm_layer. Normalize the repr by stripping the bias= field so the test passes on PyTorch versions with or without it (backward- and forward-compatible). Signed-off-by: Hans Johnson <hans-johnson@uiowa.edu>
hjmjohnson
force-pushed
the
norm-layer-repr-pytorch-compat
branch
from
c80fc22 to
19d7f35
Compare
|
Actionable comments posted: 0 |
There was a problem hiding this comment.
Hi @hjmjohnson thanks for this fix. I can look into enabling PyTorch 2.13 testing later, I know this was one problem encountered but there might have been others.
2 participants
PyTorch >= 2.13 adds an optional 'bias=' token to GroupNorm/InstanceNorm repr, breaking the exact-string match in test_norm_layer. Normalize the repr by stripping the bias= field so the test passes on PyTorch versions with or without it (backward- and forward-compatible).
Description
test_norm_layer in tests/networks/layers/test_get_layers.py asserts that a norm layer's repr() exactly equals a hard-coded string. PyTorch >= 2.13 added an optional bias= token to GroupNorm /
InstanceNorm repr, which breaks that exact-string match. This normalizes both the actual and expected repr by stripping the bias= field, so the test passes on PyTorch versions with or without
it — backward- and forward-compatible, with no change to production code.
Types of changes
PyTorch < 2.13: GroupNorm(1, 1, eps=1e-05, affine=True)
PyTorch >= 2.13: GroupNorm(1, 1, eps=1e-05, affine=True, bias=True)
The fix strips , bias=(True|False) from both sides before comparison. Local checks: targeted test 7/7 OK; black/isort/ruff clean; full pre-commit hooks pass.